-
Notifications
You must be signed in to change notification settings - Fork 3.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix auto type column indexer realtime logical type to report long if all nulls since it is most restrictive type #16828
base: master
Are you sure you want to change the base?
Conversation
…all nulls to match serialized segment if all null values
// we didn't see anything, so we can be anything, so why not a string? | ||
return ColumnType.STRING; | ||
// we didn't see anything, so we can be anything, so be long as it is the most restrictive type | ||
return ColumnType.LONG; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this column is seen previously, should it just return that type?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the issue here is that there have been no values other than nulls so far, so we can't know a type yet
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the scenario when the ingestion spec doesn't have any custom dimensions, new incremental index is initialized with columns from previous index https://github.com/apache/druid/blob/master/server/src/main/java/org/apache/druid/segment/realtime/sink/Sink.java#L379, in that case it would probably be better to return the previous seen type?
…constant-null-realtime-type
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the fix, LGTM!
Report logical type as
LONG
instead ofSTRING
sinceLONG
is the more restrictive type.